Hey! Your PR title ci: add post-deploy smoke test workflow doesn't follow conventional commit format.

Please update it to start with one of:

• feat: or feat(scope): new feature
• fix: or fix(scope): bug fix
• docs: or docs(scope): documentation changes
• chore: or chore(scope): maintenance tasks
• refactor: or refactor(scope): code refactoring
• test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

The following comment was made by an LLM, it may be inaccurate:

Claude Single-Pass Review

Summary

This PR adds a workflow_run-triggered smoke test workflow and corrects the runner for build-push.yml from an unavailable Blacksmith runner to ubuntu-latest. The workflow structure is sound, but the smoke test script has two bugs that will cause spurious test failures in CI.

Findings

[FINDING-1] issue: P1 | scripts/ci/test-opencode-integration.sh:46 | Test 3 comparison has a trailing space in the string literal — comparing against "200 " (with space) instead of "200" means this test will always fail. Verified via Read: if [ "$HTTP_CODE" = "200 " ]; then. Fix: Remove the trailing space, change "200 " to "200".

[FINDING-2] issue: P1 | scripts/ci/test-opencode-integration.sh:60 | Typo HTTP_CDE (missing P) in the Test 4 failure message — the variable is unset and will expand to empty or cause an error under set -u. Verified via Read: fail "Session creation expected 200, got ${HTTP_CDE}". Fix: Change HTTP_CDE to HTTP_CODE.

[FINDING-3] nit: P2 | .github/workflows/build-push.yml:14 | Runner changed to ubuntu-latest but project convention calls for ubicloud-standard-2. Consider switching once Ubicloud is available for this fork, or document the exception.

[FINDING-4] nit: P2 | .github/workflows/smoke-test.yml:6 | workflow_run fires from the default branch workflow definition, not the PR branch. Changes here will not take effect until merged — worth noting for future contributors.

Code Quality

• Workflow trigger logic is correct (conclusion == success guard)
• OPENCODE_URL passed as env var; no secrets required
• timeout-minutes: 5 bounds runaway tests
• issue: Test 3 comparison has trailing space — will always fail (FINDING-1)
• issue: Typo HTTP_CDE in error message variable (FINDING-2)
• set -euo pipefail present for safe script execution
• checkout@v4 pinned to major version

Recommendation

[ ] Approve | [ ] Approve with changes | [x] Request changes

Claude Review - Out-of-Diff Findings

The following findings are on lines outside the PR diff:

scripts/ci/test-opencode-integration.sh:46
[FINDING-1] issue: P1 | Test 3 comparison has a trailing space in the string literal — comparing against "200 " (with space) instead of "200" means this test will always fail. Verified via Read: if [ "$HTTP_CODE" = "200 " ]; then. Fix: Remove the trailing space, change "200 " to "200".

scripts/ci/test-opencode-integration.sh:60
[FINDING-2] issue: P1 | Typo HTTP_CDE (missing P) in the Test 4 failure message — the variable is unset and will expand to empty or cause an error under set -u. Verified via Read: fail "Session creation expected 200, got ${HTTP_CDE}". Fix: Change HTTP_CDE to HTTP_CODE.

.github/workflows/build-push.yml:14
[FINDING-3] nit: P2 | Runner changed to ubuntu-latest but project convention calls for ubicloud-standard-2. Consider switching once Ubicloud is available for this fork, or document the exception.

.github/workflows/smoke-test.yml:6
[FINDING-4] nit: P2 | workflow_run fires from the default branch workflow definition, not the PR branch. Changes here will not take effect until merged — worth noting for future contributors.

Claude Single-Pass Review

Summary

This PR adds a workflow_run-triggered smoke test for the Fly.io deployment, which is a solid CI quality-of-life improvement. However, the PR has a merge conflict because the base branch (dev) already updated build-push.yml to use ubicloud-standard-2, making the ubuntu-latest runner change in this PR a regression if merged as-is. The smoke-test workflow itself is correctly structured.

Findings

[FINDING-1] issue: P1 | .github/workflows/build-push.yml:14 | Runner regresses to ubuntu-latest but the base branch dev already has runs-on: ubicloud-standard-2. Merging this PR as-is will switch back from the Ubicloud runner to a GitHub-hosted runner, increasing CI cost. Verified via Read: the PR branch has runs-on: ubuntu-latest while dev head already has runs-on: ubicloud-standard-2. Fix: Remove the build-push.yml change from this PR entirely — the runner fix is already on dev. Resolve the merge conflict by keeping ubicloud-standard-2.

[FINDING-2] nit: P2 | .github/workflows/smoke-test.yml:20 | timeout-minutes is correctly set to 5 at the step level, but there is no job-level timeout-minutes fallback. If checkout hangs before the smoke test step, the workflow could run until GitHub's 6-hour default timeout. Consider adding timeout-minutes: 10 at the job level as a safety net.

Code Quality

• workflow_run trigger correctly filters on conclusion == 'success' — won't run on failed builds
• OPENCODE_URL injected via env var, no secrets required for public endpoint
• Uses ubicloud-standard-2 runner for smoke test job (consistent with project convention)
• actions/checkout@v4 pinned to major version
• Script invoked via bash (no execute-bit assumption in CI)
• issue: build-push.yml runner reverts a runner already fixed on base branch (FINDING-1)

Recommendation

[ ] Approve | [ ] Approve with changes | [x] Request changes

Codex Review

Summary

PR scope is small and focused, but the workflow currently validates post-build state rather than post-deploy state. That mismatch can produce false-positive smoke results for the intended goal.

Findings

[FINDING-1] issue: P1 | .github/workflows/smoke-test.yml:4 | Trigger is workflow_run of Build and Push Docker Image, which is a build/push completion signal, not a deploy completion signal. Because the smoke target is a fixed live URL (.github/workflows/smoke-test.yml:19), the check can pass while production still runs a previous image.
Fix: Trigger from the actual deploy workflow/event, or gate smoke execution on explicit deployed-version verification for the just-built tag before running endpoint checks.

Code Quality

• Types correct
• Error handling pattern acceptable for shell workflow context
• No duplication in this diff
• Naming is clear and consistent
• No debug code

Architecture

• issue: Smoke test lifecycle is not aligned with deployment lifecycle (post-build vs post-deploy).

PR Metadata

Suggested PR Title: ci: add post-build smoke test workflow (unless trigger is rewired to deploy completion)
Suggested Description Update: Clarify exact trigger source and how the workflow guarantees it validates the newly deployed artifact.

Recommendation

• Approve
• Approve with changes
• Request changes

Escalate to Gemini?

• Yes - [reason]
• No

Gemini Deep Review

Summary

The PR introduces a post-deploy smoke test workflow that validates the headless OpenCode server after successful Docker image builds. While the workflow follows project runner conventions, there are risks regarding deployment synchronization and authentication that could lead to false negatives or test failures in production.

Findings

[gemini-1] issue: P1 | .github/workflows/smoke-test.yml:16 | Synchronization Risk: The smoke test triggers immediately after the Docker image is pushed to the registry. However, build-push.yml does not appear to perform a fly deploy. If the Fly.io app does not automatically redeploy on registry push (which is standard for Fly Machines), this workflow will validate the previous version of the application instead of the new one.
Fix: Ensure a deployment step exists before the smoke test, or confirm the Fly.io app is configured for automatic redeploy on push.

[gemini-2] issue: P1 | .github/workflows/smoke-test.yml:19 | Authentication Gap: The pistachiorama-opencode server likely implements basic authentication (as seen in packages/opencode/src/server/server.ts). The smoke test script test-opencode-integration.sh (Tests 2-4) will likely fail with 401 Unauthorized because no credentials are provided via the OPENCODE_SERVER_PASSWORD environment variable.
Fix: Add a secret for the server password and pass it to the integration script.

[gemini-3] nit: P2 | .github/workflows/smoke-test.yml:18 | Hardcoded URL: The OPENCODE_URL is hardcoded to the production endpoint. While acceptable for a simple smoke test, it prevents the workflow from being used against staging or ephemeral environments.
Suggestion: Move the URL to a repository variable or environment secret.

PR Metadata

Suggested PR Title: ci: add post-deploy smoke test for Fly.io
Suggested Description Update: Added a smoke test workflow triggered by successful Build and Push events to verify health and session API endpoints on the production Fly.io deployment.

Questions

• Is the pistachiorama-opencode app configured to automatically redeploy when a new image is pushed to registry.fly.io?
• Does the production instance require Basic Auth? If so, should we add a secret for the smoke test to use?
• Why is the smoke test script scripts/ci/test-opencode-integration.sh not included in this PR? (It was observed to exist on disk but is not in the diff).

Recommendation

[ ] Approve | [x] Approve with changes | [ ] Request changes

Note: Verify the auth and deployment sync before merging to avoid noisy CI failures on every release.

Resolution Summary

Resolving findings from Codex Review, Gemini Review, Claude Review:

Codex Review

Summary

The previous blocker is addressed: deployment now occurs inside Build and Push Docker Image, so smoke-test runs only after a successful deploy path. I do not see new blocking issues in the updated diff.

Findings

None.

Code Quality

• Types correct (N/A for this YAML-only diff)
• No debug code
• Naming is clear and consistent
• Changes are focused

Architecture

• Smoke test is now aligned with deployment lifecycle (deploy is part of the upstream workflow that gates workflow_run success)
• No architecture drift introduced in this diff

PR Metadata

Suggested PR Title: ci: add post-deploy smoke test workflow
Suggested Description Update: Mention that deploy is now part of build-push.yml, and smoke tests are triggered only after successful build+deploy completion.

Recommendation

• Approve
• Approve with changes
• Request changes

Escalate to Gemini?

• Yes - [reason]
• No

Gemini Deep Review

Summary

The PR introduces a deployment step to Fly.io and a post-deployment smoke test. While the integration of these steps is a positive step for CI/CD, the smoke test is currently fragile due to the lack of retries for cold-starting Fly machines and fails to verify that the correctly tagged version was actually deployed.

Findings

[gemini-1] blocking: P0 | scripts/ci/test-opencode-integration.sh:30 | No retries for initial health check.
Verified via fly.toml: auto_stop_machines = "stop".
With machines configured to stop when idle, the first request from the smoke test will likely trigger a cold start. The current script makes a single curl call which will fail or time out if the machine is still booting, leading to flaky CI runs.
Fix: Implement a retry loop (e.g., 5 attempts with a 5s delay) for the initial health check.

[gemini-2] issue: P1 | scripts/ci/test-opencode-integration.sh:31 | Health check doesn't verify deployment version.
Verified via packages/opencode/src/server/routes/global.ts:39: return c.json({ healthy: true, version: Installation.VERSION }).
The smoke test only checks for "healthy":true but ignores the version field. This prevents detecting scenarios where the deployment failed silently or was delayed, leaving an old version running.
Fix: Pass ${{ github.event.workflow_run.head_branch }} (the tag) to the script and verify it matches the version in the JSON response.

[gemini-3] issue: P1 | .github/workflows/smoke-test.yml:19 | Smoke test URL is hardcoded.
The OPENCODE_URL is hardcoded to https://pistachiorama-opencode.fly.dev. This creates a maintenance burden and a risk of checking the wrong environment if the app name in fly.toml is updated.
Fix: Derive the URL from the app name in fly.toml or use a shared secret/variable.

[gemini-4] nit: P2 | scripts/ci/test-opencode-integration.sh:30 | Suppressing curl errors makes debugging difficult.
The use of 2>/dev/null in the curl command hides the specific reason for failures (e.g., "Connection refused" vs "Connection timeout"), which is critical for debugging infrastructure issues in CI.

[gemini-5] nit: P2 | .github/workflows/build-push.yml:52 | Unpinned flyctl installation.
Installing flyctl via curl | sh always pulls the latest version.
Fix: Pin the version or use the official superfly/flyctl-actions/setup-flyctl@master.

PR Metadata

Suggested PR Title: ci: deploy to Fly.io and add robust post-deploy smoke test
Suggested Description Update: Added Fly.io deployment to build-push.yml and a new smoke-test.yml workflow that triggers after deployment. Included a bash script to verify health and API endpoints.

Questions

• Is there a reason why the smoke test is a separate workflow rather than a final step in build-push.yml? Using workflow_run makes it harder to block a "successful" status on the primary workflow if the smoke test fails.

Recommendation

[ ] Approve | [ ] Approve with changes | [x] Request changes

Resolution Summary

Resolving findings from Gemini Review:

Codex Review

Summary

This PR adds automated deploy + smoke coverage for tagged releases, but there is one critical CI security risk and one release-validation reliability gap that should be fixed before merge.

Findings

[FINDING-1][codex-1] blocking: P0 | .github/workflows/build-push.yml:53 | superfly/flyctl-actions/setup-flyctl@master uses a mutable ref in a privileged deploy job (FLY_API_TOKEN), creating supply-chain risk.
Fix: Pin the action to an immutable commit SHA (or trusted version tag) and update intentionally.

[FINDING-2][codex-2] issue: P1 | .github/workflows/smoke-test.yml:4 | workflow_run smoke checks are not deterministically bound to the specific release being validated; concurrent tag releases can produce false pass/fail attribution.
Fix: Add release/environment concurrency and/or assert the expected deployed version from github.event.workflow_run before passing smoke.

Code Quality

• Types correct
• Error handling pattern applicable
• No duplication in diff
• Clear naming
• No debug code

Architecture

• issue: Mutable third-party action ref in privileged deployment workflow at .github/workflows/build-push.yml:53
• issue: Smoke validation not deterministically tied to triggering release at .github/workflows/smoke-test.yml:4

PR Metadata

Suggested PR Title: ci: deploy tagged image to Fly and run post-deploy smoke test
Suggested Description Update: Clarify release concurrency assumptions and how smoke verifies the intended deployed tag/version.

Recommendation

[ ] Approve | [ ] Approve with changes | [x] Request changes

Escalate to Gemini?

[x] Yes - security-sensitive CI/deploy workflow changes (privileged token + mutable action ref) | [ ] No

Gemini Deep Review

Summary

The PR introduces a post-deploy smoke test for the Fly.io deployment, but it contains a critical authentication gap that will cause the tests to fail on a secured server, and a trigger limitation that prevents verification within the PR itself.

Findings

[gemini-1] blocking: P0 | scripts/ci/test-opencode-integration.sh | Missing Authentication support.
The OpenCode server requires HTTP Basic Auth if OPENCODE_SERVER_PASSWORD is set (as enforced in packages/opencode/src/server/server.ts). The smoke test script uses curl without any Authorization headers or -u flags.
Verified via read_file:

HTTP_CODE=$(curl -sf -o "${TMPDIR_RUN}/health-response.json" -w "%{http_code}" "${BASE_URL}/global/health" 2>/dev/null || echo "000")

Fix: Update the script to accept OPENCODE_SERVER_PASSWORD and pass it to curl (e.g., curl -u opencode:$OPENCODE_SERVER_PASSWORD). Update the workflow to provide this secret.

[gemini-2] blocking: P0 | Dockerfile:64 | Security Gap: Unsecured public server.
The Dockerfile starts the server on 0.0.0.0:8080 without ensuring OPENCODE_SERVER_PASSWORD is required. Since pistachiorama-opencode.fly.dev is publicly accessible and the server provides powerful tools (bash, edit), running without mandatory auth is a critical risk.
Fix: Ensure OPENCODE_SERVER_PASSWORD is set in the Fly.io environment.

[gemini-3] issue: P1 | .github/workflows/smoke-test.yml:13 | Trigger limitation.
The workflow_run trigger only functions for workflows residing on the default branch (dev). Since this is a new file in this PR, the smoke test will not trigger for the current PR's tag pushes until after it is merged to dev.
Fix: Consolidate the smoke test as a final job in .github/workflows/build-push.yml to ensure it runs in the same context and is verifiable within the PR.

[gemini-4] issue: P1 | .github/workflows/smoke-test.yml:18 | Hardcoded production URL.
The URL https://pistachiorama-opencode.fly.dev is hardcoded. This is brittle and makes it difficult to run smoke tests against staging or preview environments.
Fix: Derive the URL from the app name or use a GitHub Environment variable.

[gemini-5] suggestion: P2 | scripts/ci/test-opencode-integration.sh:34 | Missing curl retries.
The Fly.io app uses auto_stop_machines = "stop". The first smoke test request may fail or timeout while the machine wakes up.
Fix: Add --retry 5 --retry-delay 5 --retry-connrefused to curl commands in the script.

[gemini-6] nit: P2 | .github/workflows/smoke-test.yml:1 | Inconsistent naming.
Workflow name smoke-test should be Smoke Test to match project conventions (e.g., Build and Push Docker Image).

PR Metadata

Suggested PR Title: ci: add authenticated post-deploy smoke test and Fly.io deployment
Suggested Description Update: Note that OPENCODE_SERVER_PASSWORD must be configured in Fly.io secrets for the smoke test to pass and the server to be secured.

Questions

• Is the pistachiorama-opencode Fly app intended to be publicly accessible without authentication?
• Why use workflow_run instead of a sequential job in the build-push workflow?

Recommendation

[ ] Approve | [ ] Approve with changes | [x] Request changes

Resolution Summary

Resolving findings from Codex Review, Gemini Review:

Codex Review

Summary

PR #18 is focused and appropriately scoped for CI/deploy automation. I found one must-fix determinism issue in the new workflow_run smoke test job.

Findings

[codex-1][FINDING-1] issue: P1 | .github/workflows/smoke-test.yml:14 | actions/checkout@v4 in a workflow_run workflow checks out the default branch tip by default, not the commit/tag from the triggering build, so smoke tests can run against different code than what was just deployed.
Fix: Pin checkout to the triggering SHA:

- uses: actions/checkout@v4
  with:
    ref: ${{ github.event.workflow_run.head_sha }}

Code Quality

• Types correct
• No duplication/debug code in diff
• issue: Non-deterministic checkout source in workflow_run at .github/workflows/smoke-test.yml:14

Architecture

• Follows existing CI workflow patterns
• issue: Post-deploy validation is not commit-pinned, reducing deploy-to-test traceability

PR Metadata

Suggested PR Title: ci: deploy image to Fly.io and add post-deploy smoke test
Suggested Description Update: Clarify that smoke tests should run against the same SHA/tag as the triggering deploy by pinning actions/checkout to github.event.workflow_run.head_sha.

Recommendation

[ ] Approve | [ ] Approve with changes | [x] Request changes

Escalate to Gemini?

[ ] Yes - [reason] | [x] No

Gemini Deep Review

Summary

The PR implements a post-deploy smoke test workflow that triggers after a successful Docker build and push (on version tags). It adds a Fly.io deployment step to the build-push workflow and a new smoke-test workflow that executes basic API integration tests against the production URL.

Findings

[gemini-1] nit: P2 | .github/workflows/smoke-test.yml:15 | Style inconsistency in action reference
There is an extra space before the version tag in the checkout action.

- uses: actions/checkout @v4

Verified via context: Other workflows like ci.yml and build-push.yml use actions/checkout@v4 without the space.

[gemini-2] issue: P1 | scripts/ci/test-opencode-integration.sh:38 | Brittle JSON parsing in smoke test script
The script uses grep -o '"healthy":true' to verify the health check response. This will fail if the server response contains any whitespace (e.g., { "healthy": true }).

HEALTHY=$(grep -o '"healthy":true' "${TMPDIR_RUN}/health-response.json" || true)

Fix: Use jq which is already available in the project's CI environment.

HEALTHY=$(jq -r '.healthy' "${TMPDIR_RUN}/health-response.json")
if [ "$HEALTHY" = "true" ]; then ...

[gemini-3] issue: P1 | scripts/ci/test-opencode-integration.sh:78 | Brittle regex for session ID extraction
The script uses a complex grep | head | cut chain to extract the session ID, which is prone to failure if the JSON structure changes or formatting differs.

SESSION_ID=$(grep -o '"id":"[^"]*"' "${TMPDIR_RUN}/session-create.json" | head -1 | cut -d'"' -f4 || true)

Fix: Use jq for robust extraction.

SESSION_ID=$(jq -r '.id' "${TMPDIR_RUN}/session-create.json")

[gemini-4] question: P2 | scripts/ci/test-opencode-integration.sh:69 | Missing Authentication in Smoke Test
The OpenCode server supports Basic Auth via OPENCODE_SERVER_PASSWORD (see packages/opencode/src/server/server.ts:65). If the production deployment on Fly.io has authentication enabled, the smoke test will fail with 401 Unauthorized as it does not provide credentials.
Fix: Ensure the smoke test has access to the required credentials or clarify if the test target is intentionally public.

[gemini-5] suggestion: P2 | scripts/ci/test-opencode-integration.sh | Lack of session cleanup
The smoke test creates a new session on every run (POST /session) but never deletes it. Since the Fly.io deployment uses a persistent volume (opencode_data), these sessions will accumulate indefinitely.
Suggestion: Add a DELETE /session/${SESSION_ID} step at the end of the script to prevent database bloat.

PR Metadata

Suggested PR Title: ci: add fly.io deployment and post-deploy smoke test

Questions

• Is the pistachiorama-opencode production environment intended to be public, or should the smoke test be updated to handle Basic Auth?
• Should we add a cleanup step to the smoke test to remove the temporary session created during verification?

Recommendation

[ ] Approve | [x] Approve with changes | [ ] Request changes

Resolution Summary

Resolving findings from Codex Review, Gemini Review:

Dual-Review Summary (Round 5)

Convergence: ❌ Not achieved (3 blocking findings)

Rounds: 5
Duration: 41 minutes

Finding Classification: NEW: 0 | RE-RAISED: 1

Findings

[Codex #1] P1 - .github/workflows/smoke-test.yml:14

actions/checkout@v4 in a workflow_run workflow checks out the default branch tip by default, not the commit/tag from the triggering build, so smoke tests can run against different code than what was just deployed.

[Gemini #1] P2 - .github/workflows/smoke-test.yml:15

Style inconsistency in action reference

[Gemini #2] P1 - scripts/ci/test-opencode-integration.sh:38

Brittle JSON parsing in smoke test script

[Gemini #3] P1 - scripts/ci/test-opencode-integration.sh:78

Brittle regex for session ID extraction

[Gemini #4] P2 - scripts/ci/test-opencode-integration.sh:69

Missing Authentication in Smoke Test

[Gemini #5] P2 - scripts/ci/test-opencode-integration.sh

Lack of session cleanup